feat: mark untagged manifests as garbage during GC and delete #680

wangxiaoxuan273 · 2024-01-18T07:47:10Z

Related to #664

codecov · 2024-01-18T07:48:46Z

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

Comparison is base (82cc505) 75.78% compared to head (423f3ed) 75.85%.

Files	Patch %	Lines
content/oci/oci.go	66.03%	12 Missing and 6 partials ⚠️
internal/manifestutil/parser.go	81.25%	2 Missing and 1 partial ⚠️
internal/resolver/memory.go	93.87%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #680      +/-   ##
==========================================
+ Coverage   75.78%   75.85%   +0.07%     
==========================================
  Files          59       59              
  Lines        5769     5873     +104     
==========================================
+ Hits         4372     4455      +83     
- Misses       1026     1040      +14     
- Partials      371      378       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

internal/resolver/memory_test.go

content/oci/oci.go

Wwwsylvia

LGTM with a minor suggestion

content/oci/oci.go

shizhMSFT

LGTM

internal/resolver/memory.go

content/oci/oci.go

eiffel-fl

Hi!

Thank you for this PR!
As advised in #664, I tested this PR in a branch:

francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ sudo -E ./ig image build -t opens -f gadgets/trace_open/gadget.yaml gadgets/trace_open
INFO[0000] Experimental features enabled                
Successfully built ghcr.io/inspektor-gadget/gadget/opens:latest@sha256:a5de3655d6c7640eb6d43f7d9d7182b233ac86aedddfe6c132cba6b876264d97
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ sudo cat /var/lib/ig/oci-store/index.json | jq             francis/rmi-upstream % u=
{
  "schemaVersion": 2,
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.index.v1+json",
      "digest": "sha256:a5de3655d6c7640eb6d43f7d9d7182b233ac86aedddfe6c132cba6b876264d97",
      "size": 491,
      "annotations": {
        "org.opencontainers.image.ref.name": "ghcr.io/inspektor-gadget/gadget/opens:latest"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:6fc908439cd210687651e36693d4fa96e59259403826c7cf929ac5382c8d6a33",
      "size": 582,
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:b4c03573365a929cbde0bf6e92ed91e067647501851a6d84a6f2a1a072bf22ce",
      "size": 582,
      "platform": {
        "architecture": "arm64",
        "os": "linux"
      }
    }
  ]
}
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ sudo -E ./ig image remove opens                            francis/rmi-upstream % u=
INFO[0000] Experimental features enabled                
Successfully removed opens
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ sudo cat /var/lib/ig/oci-store/index.json | jq             francis/rmi-upstream % u=
{
  "schemaVersion": 2,
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.index.v1+json",
      "digest": "sha256:a5de3655d6c7640eb6d43f7d9d7182b233ac86aedddfe6c132cba6b876264d97",
      "size": 491
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:6fc908439cd210687651e36693d4fa96e59259403826c7cf929ac5382c8d6a33",
      "size": 582,
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:b4c03573365a929cbde0bf6e92ed91e067647501851a6d84a6f2a1a072bf22ce",
      "size": 582,
      "platform": {
        "architecture": "arm64",
        "os": "linux"
      }
    }
  ]
}

As a result, the tag was just removed from the index but nothing no blobs were actually removed.
Am I missing something?

With regard to the code itself, I found two nits.

Best regards.

internal/graph/memory.go

internal/manifestutil/parser.go

wangxiaoxuan273 · 2024-01-23T07:33:58Z

Hi!

Thank you for this PR! As advised in #664, I tested this PR in a branch:

francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ sudo -E ./ig image build -t opens -f gadgets/trace_open/gadget.yaml gadgets/trace_open
INFO[0000] Experimental features enabled                
Successfully built ghcr.io/inspektor-gadget/gadget/opens:latest@sha256:a5de3655d6c7640eb6d43f7d9d7182b233ac86aedddfe6c132cba6b876264d97
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ sudo cat /var/lib/ig/oci-store/index.json | jq             francis/rmi-upstream % u=
{
  "schemaVersion": 2,
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.index.v1+json",
      "digest": "sha256:a5de3655d6c7640eb6d43f7d9d7182b233ac86aedddfe6c132cba6b876264d97",
      "size": 491,
      "annotations": {
        "org.opencontainers.image.ref.name": "ghcr.io/inspektor-gadget/gadget/opens:latest"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:6fc908439cd210687651e36693d4fa96e59259403826c7cf929ac5382c8d6a33",
      "size": 582,
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:b4c03573365a929cbde0bf6e92ed91e067647501851a6d84a6f2a1a072bf22ce",
      "size": 582,
      "platform": {
        "architecture": "arm64",
        "os": "linux"
      }
    }
  ]
}
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ sudo -E ./ig image remove opens                            francis/rmi-upstream % u=
INFO[0000] Experimental features enabled                
Successfully removed opens
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ sudo cat /var/lib/ig/oci-store/index.json | jq             francis/rmi-upstream % u=
{
  "schemaVersion": 2,
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.index.v1+json",
      "digest": "sha256:a5de3655d6c7640eb6d43f7d9d7182b233ac86aedddfe6c132cba6b876264d97",
      "size": 491
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:6fc908439cd210687651e36693d4fa96e59259403826c7cf929ac5382c8d6a33",
      "size": 582,
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:b4c03573365a929cbde0bf6e92ed91e067647501851a6d84a6f2a1a072bf22ce",
      "size": 582,
      "platform": {
        "architecture": "arm64",
        "os": "linux"
      }
    }
  ]
}

As a result, the tag was just removed from the index but nothing no blobs were actually removed. Am I missing something?

With regard to the code itself, I found two nits.

Best regards.

Hi @eiffel-fl,

If I understand your scenario correctly, when you delete an image index, you also want the manifests in the index to be automatically removed, given that the manifests are untagged and do not have other tagged predecessors.

I have added a simple unit test to cover the scenario of deleting an image index. It looks like it's working as expected. In this unit test, untagged manifests are all removed and the tagged one remains.

I see that in your branch, you replaced deleteTree (which deletes all successors of root if they have no other predecessors?) with Delete and GC. Logically it should work. Do the manifests have no other tagged predecessors, other than the image index?

eiffel-fl

Hi!

I see that in your branch, you replaced deleteTree (which deletes all successors of root if they have no other predecessors?) with Delete and GC. Logically it should work. Do the manifests have no other tagged predecessors, other than the image index?

I took another look and something is wrong in my code, we modified it recently and I guess the modification had a side effect.
So, I tested the Delete(); GC() snippet only and got what I expected:

$ sudo -E ./ig image build -t opens -f gadgets/trace_open/gadget.yaml gadgets/trace_open
INFO[0000] Experimental features enabled                
Successfully built ghcr.io/inspektor-gadget/gadget/opens:latest@sha256:a5de3655d6c7640eb6d43f7d9d7182b233ac86aedddfe6c132cba6b876264d97
$ sudo ls /var/lib/ig/oci-store/blobs/sha256                francis/rmi-upstream *% u=
35166e636330705738ad3cb162b38aeffe639b718caaee63efe777f510bd2869  a5de3655d6c7640eb6d43f7d9d7182b233ac86aedddfe6c132cba6b876264d97
6fc908439cd210687651e36693d4fa96e59259403826c7cf929ac5382c8d6a33  b4c03573365a929cbde0bf6e92ed91e067647501851a6d84a6f2a1a072bf22ce
a1b5ae61c307cb52fb0480a1bc9b422ef05d1c0e80c9688d65db6cf4e9e7cb31  f10d65e6c0eadaf823f57a5733c9f2bdc444f4c8e4ec77452ba2cfd2c5e258f4
$ sudo cat /var/lib/ig/oci-store/index.json | jq            francis/rmi-upstream *% u=
{
  "schemaVersion": 2,
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.index.v1+json",
      "digest": "sha256:a5de3655d6c7640eb6d43f7d9d7182b233ac86aedddfe6c132cba6b876264d97",
      "size": 491,
      "annotations": {
        "org.opencontainers.image.ref.name": "ghcr.io/inspektor-gadget/gadget/opens:latest"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:6fc908439cd210687651e36693d4fa96e59259403826c7cf929ac5382c8d6a33",
      "size": 582,
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:b4c03573365a929cbde0bf6e92ed91e067647501851a6d84a6f2a1a072bf22ce",
      "size": 582,
      "platform": {
        "architecture": "arm64",
        "os": "linux"
      }
    }
  ]
}
$ sudo -E ./ig image list                                   francis/rmi-upstream *% u=
INFO[0000] Experimental features enabled                
REPOSITORY                                                     TAG                                                           DIGEST      
opens                                                          latest                                                        a5de3655d6c7
$ sudo -E ./ig image remove opens                           francis/rmi-upstream *% u=
INFO[0000] Experimental features enabled                
Successfully removed opens
$ sudo cat /var/lib/ig/oci-store/index.json | jq            francis/rmi-upstream *% u=
{
  "schemaVersion": 2,
  "manifests": null
}
$ sudo ls /var/lib/ig/oci-store/blobs/sha256                francis/rmi-upstream *% u=
$ git diff
diff --git a/go.mod b/go.mod
index 1d5560ba..0a6efeae 100644
--- a/go.mod
+++ b/go.mod
@@ -49,7 +49,7 @@ require (
        github.com/hashicorp/go-multierror v1.1.1
        github.com/kr/pretty v0.3.1
        github.com/moby/moby v24.0.7+incompatible
-       github.com/opencontainers/image-spec v1.1.0-rc5
+       github.com/opencontainers/image-spec v1.1.0-rc6
        github.com/prometheus/client_golang v1.18.0
        github.com/shopspring/decimal v1.3.1
        github.com/spf13/viper v1.18.2
@@ -204,4 +204,4 @@ replace sigs.k8s.io/controller-runtime => sigs.k8s.io/controller-runtime v0.13.1
 
 replace github.com/vishvananda/netns => github.com/inspektor-gadget/netns v0.0.5-0.20230524185006-155d84c555d6
 
-replace oras.land/oras-go/v2 => github.com/wangxiaoxuan273/oras-go/v2 v2.0.0-20240119103641-f8911ebadde6
+replace oras.land/oras-go/v2 => github.com/wangxiaoxuan273/oras-go/v2 v2.0.0-20240123071211-5650fa5bbcc2

So, thank you a lot for the code!

Best regards.

content/oci/oci.go

shizhMSFT

LGTM

content/oci/oci.go

shizhMSFT

LGTM

Wwwsylvia

LGTM

Signed-off-by: Xiaoxuan Wang <[email protected]>

following #680 and addressing this comment: #680 (comment) Signed-off-by: Xiaoxuan Wang <[email protected]>

wangxiaoxuan273 requested review from sajayantony, shizhMSFT, SteveLasker and Wwwsylvia as code owners January 18, 2024 07:47

wangxiaoxuan273 force-pushed the demo branch from 4c5c8ad to 08fcd2e Compare January 18, 2024 07:49

Wwwsylvia requested changes Jan 18, 2024

View reviewed changes

internal/resolver/memory_test.go Outdated Show resolved Hide resolved

content/oci/oci.go Outdated Show resolved Hide resolved

Wwwsylvia reviewed Jan 18, 2024

View reviewed changes

content/oci/oci.go Show resolved Hide resolved

Wwwsylvia mentioned this pull request Jan 18, 2024

Manifest should not be added to index.json #664

Closed

Wwwsylvia approved these changes Jan 18, 2024

View reviewed changes

content/oci/oci.go Outdated Show resolved Hide resolved

wangxiaoxuan273 mentioned this pull request Jan 19, 2024

refactor: remove AutoRemoveReferrers flag #683

Merged

shizhMSFT approved these changes Jan 19, 2024

View reviewed changes

internal/resolver/memory.go Show resolved Hide resolved

Wwwsylvia requested changes Jan 19, 2024

View reviewed changes

content/oci/oci.go Show resolved Hide resolved

eiffel-fl reviewed Jan 22, 2024

View reviewed changes

internal/graph/memory.go Show resolved Hide resolved

internal/manifestutil/parser.go Show resolved Hide resolved

wangxiaoxuan273 force-pushed the demo branch from 26388a7 to d5e6361 Compare January 23, 2024 06:04

eiffel-fl approved these changes Jan 23, 2024

View reviewed changes

shizhMSFT reviewed Jan 23, 2024

View reviewed changes

content/oci/oci.go Show resolved Hide resolved

shizhMSFT approved these changes Jan 23, 2024

View reviewed changes

Wwwsylvia reviewed Jan 23, 2024

View reviewed changes

content/oci/oci.go Show resolved Hide resolved

content/oci/oci.go Outdated Show resolved Hide resolved

shizhMSFT approved these changes Jan 23, 2024

View reviewed changes

Wwwsylvia approved these changes Jan 23, 2024

View reviewed changes

wangxiaoxuan273 added 8 commits January 23, 2024 18:08

unit test

5b009c8

Signed-off-by: Xiaoxuan Wang <[email protected]>

refinement

2ccc178

Signed-off-by: Xiaoxuan Wang <[email protected]>

renamed a function

32ee904

Signed-off-by: Xiaoxuan Wang <[email protected]>

resolved comments

d88a384

Signed-off-by: Xiaoxuan Wang <[email protected]>

refinement

d860b53

Signed-off-by: Xiaoxuan Wang <[email protected]>

index referrers

08ef665

Signed-off-by: Xiaoxuan Wang <[email protected]>

resolved comments

43699e0

Signed-off-by: Xiaoxuan Wang <[email protected]>

increase coverage

c8224eb

Signed-off-by: Xiaoxuan Wang <[email protected]>

wangxiaoxuan273 added 8 commits January 23, 2024 18:08

increase coverage

021e28d

Signed-off-by: Xiaoxuan Wang <[email protected]>

increase coverage

e090ffd

Signed-off-by: Xiaoxuan Wang <[email protected]>

refined

4900992

Signed-off-by: Xiaoxuan Wang <[email protected]>

added one more test on deleting an index

e4a5b9e

Signed-off-by: Xiaoxuan Wang <[email protected]>

refinement

028e7a9

Signed-off-by: Xiaoxuan Wang <[email protected]>

refined comments

a9aeca7

Signed-off-by: Xiaoxuan Wang <[email protected]>

increase coverage

607950c

Signed-off-by: Xiaoxuan Wang <[email protected]>

refined

423f3ed

Signed-off-by: Xiaoxuan Wang <[email protected]>

wangxiaoxuan273 force-pushed the demo branch from 075a21f to 423f3ed Compare January 23, 2024 10:08

Wwwsylvia merged commit 64fedf4 into oras-project:main Jan 23, 2024
9 checks passed

Wwwsylvia pushed a commit that referenced this pull request Jan 26, 2024

refactor: remove AutoRemoveReferrers flag (#683)

850a247

following #680 and addressing this comment: #680 (comment) Signed-off-by: Xiaoxuan Wang <[email protected]>

Wwwsylvia mentioned this pull request Jan 26, 2024

Vote for release of v2.4.0 #696

Closed

4 tasks

wangxiaoxuan273 deleted the demo branch March 6, 2024 07:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: mark untagged manifests as garbage during GC and delete #680

feat: mark untagged manifests as garbage during GC and delete #680

wangxiaoxuan273 commented Jan 18, 2024

codecov bot commented Jan 18, 2024 •

edited

Loading

Wwwsylvia left a comment

shizhMSFT left a comment

eiffel-fl left a comment

wangxiaoxuan273 commented Jan 23, 2024

eiffel-fl left a comment

shizhMSFT left a comment

shizhMSFT left a comment

Wwwsylvia left a comment

feat: mark untagged manifests as garbage during GC and delete #680

feat: mark untagged manifests as garbage during GC and delete #680

Conversation

wangxiaoxuan273 commented Jan 18, 2024

codecov bot commented Jan 18, 2024 • edited Loading

Codecov Report

Wwwsylvia left a comment

Choose a reason for hiding this comment

shizhMSFT left a comment

Choose a reason for hiding this comment

eiffel-fl left a comment

Choose a reason for hiding this comment

wangxiaoxuan273 commented Jan 23, 2024

eiffel-fl left a comment

Choose a reason for hiding this comment

shizhMSFT left a comment

Choose a reason for hiding this comment

shizhMSFT left a comment

Choose a reason for hiding this comment

Wwwsylvia left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 18, 2024 •

edited

Loading